Skip to content

test(db): gate replica routing in CI - #4596

Draft
MajorTal wants to merge 1 commit into
block:mainfrom
MajorTal:codex/issue-3622-replica-routing-ci
Draft

test(db): gate replica routing in CI#4596
MajorTal wants to merge 1 commit into
block:mainfrom
MajorTal:codex/issue-3622-replica-routing-ci

Conversation

@MajorTal

@MajorTal MajorTal commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Sorry for the noisy nesting diff. Moving these existing fixtures into a dedicated module adds indentation, but it gives Nextest an explicit, auditable selector that cannot silently miss oddly named routing tests.

This turns the hand-verified replica-routing invariants into a CI gate before BUZZ_REPLICA_READ_MAX_AGE_MS is enabled.

Related issue

Fixes #3622

Testing

  • built a buzz-db CI-profile archive with the pinned cargo-nextest 0.9.136, then ran the exact proposed archived command: 5 passed, 242 skipped
  • TEST_DATABASE_URL=postgres://buzz:buzz_dev@localhost:5432/buzz cargo test -p buzz-db --lib 'tests::replica_routing::' -- --ignored --test-threads=1: 5 passed
  • cargo test -p buzz-db --lib -- --test-threads=1: 94 passed, 153 ignored
  • cargo clippy -p buzz-db --lib --tests -- -D warnings
  • cargo fmt -p buzz-db -- --check
  • pre-push gates: branch skew, all eight Rust unit-test groups, and desktop Tauri clippy/tests

Signed-off-by: Tal Weiss <major.tal@gmail.com>
@MajorTal
MajorTal force-pushed the codex/issue-3622-replica-routing-ci branch from a364ca2 to e795553 Compare August 10, 2026 10:04
@MajorTal

Copy link
Copy Markdown
Contributor Author

Ran the acceptance criteria from #3622 locally against this branch (Postgres 17.10 in Docker, pinned cargo-nextest 0.9.136, the exact -E expression from ci.yml). Results below, including the one criterion the PR body didn't yet evidence — plus a coverage question I'd like a second opinion on.

Acceptance criteria

1. Both named fixtures run ✅ — the -E 'package(buzz-db) and test(/replica_routing::/)' selection resolves to exactly five, including the one-budget and seven-seam isolation fixtures:

buzz-db tests::replica_routing::count_events_routed_is_bounded_only
buzz-db tests::replica_routing::lazy_reader_pool_still_spawns_fence_probe
buzz-db tests::replica_routing::query_events_routed_defaults_dark_and_routes_covered_when_enabled
buzz-db tests::replica_routing::routed_fallback_spends_one_acquire_budget_when_aurora_cache_is_cold
buzz-db tests::replica_routing::routed_reads_are_confined_to_the_requested_community

2. "A revert of only the #3268 production fix fails CI" ✅ — this is the one the PR body didn't cover, so I ran it. Removing only the Err(sqlx::Error::PoolTimedOut) arm in proved_reader (lib.rs:897-901), so a saturated reader falls through to reader_validation_error:

test ...routed_fallback_spends_one_acquire_budget_when_aurora_cache_is_cold ... FAILED

assertion `left == right` failed: saturated reader must fall back as
writer/reader_acquire_timeout; got {("writer", "reader_validation_error"): 1}
  left: None
 right: Some(1)

test result: FAILED. 4 passed; 1 failed

One failure, and the right one — the other four stay green because the mutation doesn't touch what they cover. The gate discriminates rather than merely running. Mutation reverted; nothing in this branch changed.

3. No selection of the usage-metrics lock test ✅ — 0 of 5. Also worth noting for anyone re-reading #3622's Constraint 1: #3619 closed on 2026-07-30, so the ordering dependency is already satisfied.

The coverage question

#3622 Constraint 2 lists seven fixture families a selection must not miss. The module-based selection covers three:

Family In gate
query_events_routed_*
count_events_routed_*
lazy_reader_pool_*
head_fetch_routes_by_configured_budget
channel_window_routes_*
thread_replies_cursor_pages_*
read_session_degrades_*

Overall the gate selects 5 of 154 ignored buzz-db fixtures. Also still outside it: read_session_degrades_to_writer_when_replica_connection_dies, replica_window_failure_falls_back_to_writer, thread_full_replica_page_above_fence_is_reverified_on_writer, fence_probe_refuses_to_start_without_verified_floor_guard, and the floor_guard_* fixtures.

I want to be fair about this: #3622 also says "initially cover the exact routing matrix verified by hand in #3268," and this PR scopes itself to that matrix. If those five are the matrix, this is correct as scoped and the rest is follow-up work. I couldn't confirm it either way — #3268's body doesn't enumerate the matrix — so this is a question, not a defect claim.

One thing I'd suggest regardless. #3622 asks for "a comment stating it must cover every #[ignore]d routing fixture." The current comment states the converse:

every ignored fixture in tests::replica_routing is part of the replica-read activation gate

That guarantees nothing extraneous is in the module, but not that every routing fixture is in it — so a future routing fixture written outside tests::replica_routing is silently ungated, which is the same shape as the original bug. Worth either widening the module to the full routing set, or restating the comment as the obligation ("every ignored routing fixture must live in this module") so the next author knows where to put one.

Happy to do either here if you'd like — I have the environment set up and can re-run the revert check against a widened selection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI never runs the buzz-db replica-routing Postgres fixtures — enforce them before enabling BUZZ_REPLICA_READ_MAX_AGE_MS

1 participant